-
Notifications
You must be signed in to change notification settings - Fork 237
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
move viscosity averaging function #6008
Conversation
/rebuild |
/rebuild |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would aspect/material_model/utilities.h be a better place? you only want to make this accessible to other material models than just elasticity, correct?
@gassmoeller that is probably true, but it would require more code refactoring. I'm happy to either stick with what I have here or do the refactoring. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see. I am ok with keeping it in the interface for now then. However, please extend the documentation of the function similar to what I suggest.
|
||
/** | ||
* Parse an AveragingOperation and alias to an AveragingOperation | ||
* that is appropriate for viscosity averaging. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please extend the description of the function, I had to read the implementation to understand what is happening here. Something like the following:
* Parse an AveragingOperation and alias to an AveragingOperation
* that only averages viscosity. In the case of any averaging operation
* that averages all properties the corresponding operation for only
* viscosity is selected (e.g. 'harmonic_average_only_viscosity' instead
* of 'harmonic_average'). This is useful in places where averaging is
* performed manually on only the viscosity property.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 good to go if the testers are happy.
This PR moves the
get_averaging_operation_for_viscosity()
function frommaterial_model/rheology/elasticity.cc
intomaterial_model/interface.cc
.This is a more logical place for the function, and makes it more easily accessible by rheology and material_model modules.
Addresses a single comment here: #5984 (comment)